Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fields and default layer for man_made:bridge #5269

Merged
merged 3 commits into from
Sep 4, 2018

Conversation

LaszloEr
Copy link
Contributor

  • Added name, bridge type, layer and maxweight fields
  • default layer should be set to 1 (like it is for highway=bridge)

- Added name, bridge type, layer and maxweight fields
- default layer should be set to 1 (like it is for highway=bridge)
@lakedistrictOSM
Copy link

Thanks for this PR, this is a much needed improvement.

data/presets/presets/man_made/bridge.json Show resolved Hide resolved
"fields": [
"name"
"bridge"
"layer"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing commas between the array values here

@vershwal
Copy link
Collaborator

Great @LaszloEr!! Seems like some changes are required in the code. After you do the changes suggested by @SilentSpike do not forget to build the project before you push the changes. You can do it by running the command npm run build
Thanks!!

@LaszloEr
Copy link
Contributor Author

LaszloEr commented Sep 1, 2018

I added the missing commas, this is what it looks like:
bridge

@Nakaner
Copy link

Nakaner commented Sep 3, 2018

I don't think that adding a default layer=1 is a good idea. It bears the risk that users who don't care for the layer tag add a bridge which has a different layer than the road on it.

It is easier for both data consumers and mappers to find objects without a certain tag than objects where a tag which was set by default is wrong.

@LaszloEr
Copy link
Contributor Author

LaszloEr commented Sep 4, 2018

Good point. The reason why I wanted to add it as the default is because setting the bridge attribute on roads also adds it there automatically, probably because it's the correct for 99% of all bridges.

@bhousel
Copy link
Member

bhousel commented Sep 4, 2018

Thanks @LaszloEr - this seems ok to me 👍

@bhousel bhousel merged commit 64820a6 into openstreetmap:master Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants